Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[rush-lib] fix: update shrinkwrap when globalPackageExtensions has been changed #4913

Merged
merged 4 commits into from
Sep 12, 2024
Merged

[rush-lib] fix: update shrinkwrap when globalPackageExtensions has been changed #4913

merged 4 commits into from
Sep 12, 2024
+165 −1

Conversation

kenrick95
Copy link
Contributor

Summary

Fixes #4901

Details

Add one more condition in WorkspaceInstallManager to compare the values of:

  • pnpm-config.json's globalPackageExtensions checksum
  • shrinkwrap's packageExtensionsChecksum

To calculate the checksum, I added the same implementation as per what's used in pnpm (ref). The code snippet used in pnpm is as such:

const packageExtensionsChecksum = isEmpty(opts.packageExtensions ?? {}) ? undefined : createObjectChecksum(opts.packageExtensions!)


export function createObjectChecksum (obj: Record<string, unknown>) {
  const s = JSON.stringify(sortKeys(obj, { deep: true }))
  return crypto.createHash('md5').update(s).digest('hex')
}

Since it depends on sortKeys (which pnpm uses [email protected] npm package), I asked in zulip if it's okay to add a new external dependency or not. The reply I received is to implement it in node-core-library, so I added: Sort.sortKeys with the tests imported and modified from [email protected].

How it was tested

  1. Run testrush update in a Rush repo
  2. Change globalPackageExtensions
  3. Run testrush update and expect that it updates the lockfile
> testrush update

Rush Multi-Project Build Tool 5.133.3 (unmanaged) - https://rushjs.io
Node.js version is 18.20.4 (LTS)


Starting "rush update"

Checking Git policy for this repository.


Found files in the "common/git-hooks" folder.
Successfully installed these Git hook scripts: commit-msg, owners.json, post-checkout, pre-commit, pre-push

Trying to acquire lock for pnpm-8.15.8
Acquired lock for pnpm-8.15.8
Found pnpm version 8.15.8 in /path/to/user/.rush/node-v18.20.4/pnpm-8.15.8

Symlinking "/path/to/repo/common/temp/pnpm-local"
  --> "/path/to/user/.rush/node-v18.20.4/pnpm-8.15.8"
Transforming /path/to/repo/common/config/rush/.npmrc
  --> "/path/to/repo/common/temp/.npmrc"

Updating workspace files in /path/to/repo/common/temp
Copying "/path/to/repo/common/config/rush/pnpm-lock.yaml"
  --> "/path/to/repo/common/temp/pnpm-lock.yaml"
Copying "/path/to/repo/common/config/rush/pnpm-lock.yaml"
  --> "/path/to/repo/common/temp/pnpm-lock-preinstall.yaml"

The shrinkwrap file (pnpm-lock.yaml) contains the following issues:
  The package extension hash doesn't match the current shrinkwrap.


Checking installation in "/path/to/repo/common/temp"

Running "pnpm install" in /path/to/repo/common/temp

Impacted documentation

Updated the JSON schema of node-core-library

@kenrick95 kenrick95 force-pushed the kenrick/fix-update-shrinkwrap-packageExtensions branch from 34fa0f7 to 46bc5ed Compare September 6, 2024 08:23
@kenrick95 kenrick95 force-pushed the kenrick/fix-update-shrinkwrap-packageExtensions branch from 46bc5ed to f068060 Compare September 6, 2024 08:43
@kenrick95 kenrick95 requested a review from octogonz September 6, 2024 08:44
D4N14L
D4N14L reviewed Sep 6, 2024
View reviewed changes
libraries/node-core-library/src/Sort.ts Outdated Show resolved Hide resolved
libraries/node-core-library/src/Sort.ts Show resolved Hide resolved
libraries/node-core-library/src/Sort.ts Outdated Show resolved Hide resolved
libraries/node-core-library/src/Sort.ts Outdated Show resolved Hide resolved
libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts Show resolved Hide resolved
@kenrick95 kenrick95 requested a review from D4N14L September 9, 2024 03:03
dmichon-msft
dmichon-msft reviewed Sep 9, 2024
View reviewed changes
libraries/node-core-library/src/Sort.ts Outdated Show resolved Hide resolved
libraries/node-core-library/src/Sort.ts Outdated Show resolved Hide resolved
@kenrick95 kenrick95 force-pushed the kenrick/fix-update-shrinkwrap-packageExtensions branch 2 times, most recently from ea7ad02 to 9d703b6 Compare September 10, 2024 03:28
@kenrick95 kenrick95 force-pushed the kenrick/fix-update-shrinkwrap-packageExtensions branch from 9d703b6 to ed9dc1d Compare September 10, 2024 03:31
@iclanton iclanton enabled auto-merge (squash) September 12, 2024 23:47
@iclanton iclanton merged commit c6b5e1d into microsoft:main Sep 12, 2024
4 checks passed
@kenrick95 kenrick95 deleted the kenrick/fix-update-shrinkwrap-packageExtensions branch September 13, 2024 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iclanton iclanton iclanton approved these changes

@apostolisms apostolisms Awaiting requested review from apostolisms apostolisms is a code owner

@octogonz octogonz Awaiting requested review from octogonz octogonz is a code owner

@dmichon-msft dmichon-msft Awaiting requested review from dmichon-msft dmichon-msft is a code owner

@D4N14L D4N14L Awaiting requested review from D4N14L D4N14L is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Bug Triage
Status: Closed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[rush] Rush update should update shrinkwrap when globalPackageExtension has changed
5 participants
@kenrick95 @D4N14L @octogonz @iclanton @dmichon-msft